Make valid::expressions module depend on "validate" feature.
#2480
Conversation
Rather than having `#[cfg(feature = "validate")]` attributes on almost every definition in `src/valid/expressions.rs`, move the definitions we need unconditionally out of the module, and just put the whole module under a `#[cfg]`.
| @@ -243,6 +243,121 @@ pub enum ValidationError { | |||
| Corrupted, | |||
| } | |||
|
|
|||
| #[derive(Clone, Debug, thiserror::Error)] | |||
| #[cfg_attr(test, derive(PartialEq))] | |||
| pub enum ExpressionError { | |||
There was a problem hiding this comment.
Do we have to move this out? Can we make it conditional on the feature flag instead?
There was a problem hiding this comment.
The problem with making it conditional is that that changes the API of the validator. We want turning off "validate" to simply make producing ModuleInfo faster, without changing its API.
There was a problem hiding this comment.
For example, clients that happen to be checking for particular ExpressionError variants shouldn't break.
Hmm. Maybe it's preferable for any users checking for FunctionError::Expression to get a compilation error if the "validation" feature is off, since that error will never be generated and users shouldn't be thinking it could happen?
API stability and deliberate API instability both seem to have arguments in their favor. Which do you think is more useful?
There was a problem hiding this comment.
I think it's natural for feature flags to expose more of the API.
For users that turn on the flag, all enum variants of ValidationError will be present, for those who don't they may or may not be there but I don't think those users would exhaustively match the enum anyway.
|
Hello, thank you for your PR against Naga! As part of gfx-rs/wgpu#4231, we have moved development of Naga into the wgpu repository in the Naga subfolder. We have transferred all issues, but we are unable to automatically transfer PRs. As such, please recreate your PR against the wgpu repository. We apologize for the inconvenience this causes, but will make contributing to both projects more streamlined going forward. We are leaving PRs open, but once they are transferred, please close the original Naga PR. |
|
Since validation is no longer behind a feature, this PR is irrelevant. |
Rather than having
#[cfg(feature = "validate")]attributes on almost every definition insrc/valid/expressions.rs, move the definitions we need unconditionally out of the module, and just put the whole module under a#[cfg].